Move snapshot history expire table properties to constants#1217
Move snapshot history expire table properties to constants#1217sungwy merged 2 commits intoapache:mainfrom
Conversation
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
Thanks for the PR. LGTM. I think we might need to go through the deprecation steps and mark the properties in console.py as deprecated.
|
@kevinjqliu, please take a look. I have tried many solutions, and this is the best I could come up with. The problem is that any import in this module triggers the deprecation message. The lesson I learned is to never have a constant at the module level. It’s better to namespace constants within classes, so we can deprecate them by simply creating a function as a property. |
sungwy
left a comment
sungwy
left a comment
There was a problem hiding this comment.
Hi folks, sorry for taking a minute to review this PR.
I agree with @ndrluis 's assessment here, that this is the best way to deprecate these global constants.
It's not ideal that the warning message is printed when the module is imported, but at least it is being communicated clearly in a way
3 participants
I will be opening some PRs that will make it possible to implement the expire snapshot procedure. To make it easier to review, I'm breaking it down into smaller PRs.